Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

downgrade autoconf to 2.69 #508

Merged
merged 3 commits into from
Jul 29, 2021
Merged

downgrade autoconf to 2.69 #508

merged 3 commits into from
Jul 29, 2021

Conversation

Mistuke
Copy link
Collaborator

@Mistuke Mistuke commented Jul 27, 2021

Workaround for #502

I've explicitly chosen to do a downgrade (since pacman can't install specific version) so that it's easier to remove the workaround later. Just remove the code added here.

@Mistuke Mistuke requested a review from kazu-yamamoto July 27, 2021 18:09
@Mistuke
Copy link
Collaborator Author

Mistuke commented Jul 27, 2021

right, dunno anything about how github actions work, what's the caching behavior here? is it resetting my downgrade @kazu-yamamoto ?

@kazu-yamamoto
Copy link
Collaborator

I don't know github action well. I have just re-started the actions.

@kazu-yamamoto
Copy link
Collaborator

The same result, sigh.

@Mistuke
Copy link
Collaborator Author

Mistuke commented Jul 28, 2021

sigh why is it stuck now for hours in a queue..

@kazu-yamamoto
Copy link
Collaborator

I canceled and re-run this jobs.

@Mistuke
Copy link
Collaborator Author

Mistuke commented Jul 29, 2021

I canceled and re-run this jobs.

Thanks! That seems to have worked

Copy link
Collaborator

@kazu-yamamoto kazu-yamamoto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now LGTM

@kazu-yamamoto kazu-yamamoto merged commit 42e46a1 into master Jul 29, 2021
@kazu-yamamoto kazu-yamamoto deleted the autoconf-workaround branch July 29, 2021 05:06
@kazu-yamamoto
Copy link
Collaborator

Merged. Thank you for your effort!

@Mikolaj
Copy link
Member

Mikolaj commented Oct 7, 2021

Edit: I was wrong, this fails only with cabal 3.4. Still this breaks the Haskell ecosystem, since cabal 3.4. is still the default, see #513.

May I suggest to remove this workaround? It probably obscured the failure described in haskell/cabal#7707 --- basically cabal 3.4 fails with the new config.guess and config.sub files and cabal 3.6.0 that had a specific fix for them probably fails too, and cabal 3.6.2 that had a different fix definitely fails as well.

@Mistuke
Copy link
Collaborator Author

Mistuke commented Oct 7, 2021

I miss how this change which is only in network's CI file can affect your builds at all.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 7, 2021

I does not. But it masks network package failures in network package CI, which led to network being released with files that cabal is unable to consume.

@Mistuke
Copy link
Collaborator Author

Mistuke commented Oct 7, 2021

I does not. But it masks network package failures in network package CI, which led to network being released with files that cabal is unable to consume.

Hmm but there's no failure in network. The only cabal that works with autoconf 2.70+ is cabal 3.6.1.0 and up. Since the CI builds from source it would have to run autoreconf anyway. Which means that the CI can only build with tip of trunk.

Removing this means we can't test network on Windows at all, which is a worse state to be in. The question now is should the release of network be hidden on hackage until cabal 3.6.2 is released. But whatever happens here you'll never be able to compile network with cabal 3.4 again while supporting other platforms like the M1, so that goal is unrealistic.

People who want to use network on Windows are forced to use 3.6.2.0 or whatever is the one with the proper fix. Or are you saying that fix doesn't work?

@Mikolaj
Copy link
Member

Mikolaj commented Oct 7, 2021

I thought 3.6.2 doesn't work, but I was wrong: missing that our CI depends on 3.4 working and only then checks current branch.

You have a point and I was only complaining a problem was masked. Perhaps let's move this discussion to #513?

@Mistuke
Copy link
Collaborator Author

Mistuke commented Oct 7, 2021

Sure, I think there's a perfect storm here though. 3.4 can really only work if you downgrade autoconf. So if ghcup wants to still support it it needs to do that action itself.

Afterall, network isn't the only configure based package. Any user package can have the same issue. We're only focusing on Network because I guess it's widely used.

@hasufell
Copy link
Member

hasufell commented Oct 7, 2021

Moving breakage from one platform to another is not a solution.

  1. All darwin M1 needs is a simple autoreconf, no patches, nothing else
  2. As such, this problem doesn't exist on stack

So the fallout is much lower. You're gonna break a lot of CIs right now due to this new point-release. Cabal based projects on windows using cabal-3.4.0.0 will pick it up and fail.

@Mistuke
Copy link
Collaborator Author

Mistuke commented Oct 7, 2021

@hasufell This ticket is completely independent of your issue.

As I mentioned 3 times now already. Network can make a release using an older autoconf, that is completely independent from the CI and this PR. and it's also completely independent from the fact that you're choosing to stay with 3.4.

So I will repeat it again, for the 4th time. @kazu-yamamoto can make a release using an older autoconf, but YOU have to address the issue because network cannot stay on an autoconf version that will soon cycle out of msys2 anyway.

So please, do read my replies.

@Mikolaj
Copy link
Member

Mikolaj commented Oct 7, 2021

Oh dear, that's why I asked to move the discussion to #513, where it's on topic.

@Mistuke
Copy link
Collaborator Author

Mistuke commented Oct 7, 2021

Oh dear, that's why I asked to move the discussion to #513, where it's on topic.

Yeah, I'm sorry I got a bit annoyed there as well... That wasn't very helpful.

But I did mention it multiple times already... and the fact is configure packages on Windows are a very big pain. A core package such as network I don't see why it needs it on a static platform like Windows. But we don't have a solution for that atm (you can't have a package be configure on one platform and not on the other).

Its frustrating because we're just not in control of much of the ecosystem. and this failure has been a pain which has been met with silence by the autconf maintainers, which gives you an idea of how much they care.

So while I appreciate the thought, and appreciate the pain (which is why I mentioned we should do a new release to fix it). We simply cannot guarantee cabal 3.4 keeps being supported on the long run as it is. So yes, I agree because the package was a revision release we should fix it.

But for the release after this I think it's perfectly acceptable for network to bump the minor release and drop 3.4.

@hasufell
Copy link
Member

hasufell commented Oct 7, 2021

We simply cannot guarantee cabal 3.4 keeps being supported on the long run as it is.

Sure, but we have a fix outlined here #513 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants